Repository navigation
fix(web): expanding stacked notices no longer scrolls a working thread - #20086
derektrimm wants to merge 3 commits into
Conversation
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a localized web bug fix that prevents expanded stacked notices from increasing the timeline reservation and causing a later scroll jump. Existing behavior remains unchanged when the optional adjustment is absent, and the new calculation is covered by focused tests. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue [#19880] requires the end-following timeline to move with notice expansion so the last message is not covered, for idle and working threads. This change subtracts expanded notice height from the… |
Implement the [#19880] expansion behavior that keeps the last message clear for idle and working threads. Add automated coverage for expansion, the next streamed row, and collapse. |
-
Check name Status Explanation Out of Scope Changes check The changes stay within the [ #19880] composer notice and timeline layout scope.ComposerBannerStackadds the measurement marker,ChatViewexcludes expanded notice growth from the reservation, and …Title check The title clearly and concisely describes the primary fix: preventing stacked composer notices from shifting a working thread. Description check The description is complete and relevant. It explains the problem, the implementation, maintainer-approved scope, focused verification, screenshots, recordings, limitations, and CI status. The content… Full details: Out of Scope Changes check
-
Explanation
The changes stay within the [
#19880] composer notice and timeline layout scope.ComposerBannerStackadds the measurement marker,ChatViewexcludes expanded notice growth from the reservation, andcomposerFooterLayout.test.tscovers the inset calculation.
Full details: Description check
-
Explanation
The description is complete and relevant. It explains the problem, the implementation, maintainer-approved scope, focused verification, screenshots, recordings, limitations, and CI status. The content does not use the template's explicit section headings, but it contains the required information.
-
Full details: Linked Issues check
-
Explanation
Issue [
#19880] requires the end-following timeline to move with notice expansion so the last message is not covered, for idle and working threads. This change subtracts expanded notice height from the reserved inset. The added test confirms that the inset remains stable while notices expand. The implementation therefore removes the late jump after a new row, but it intentionally leaves the last rows covered during expansion. The direct issue requirement remains unmet.Resolution
Implement the [
#19880] expansion behavior that keeps the last message clear for idle and working threads. Add automated coverage for expansion, the next streamed row, and collapse.
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
00fe53e to
e7ed45a
Compare
Hovering the composer's notice stack grows the overlay, and the timeline reserved that growth as footer space. The thread did not move at once, but the next row re-pinned it to the new end, jumping by the expanded height. The reservation now leaves out how far the stack grew past its collapsed height, so working threads behave like idle ones: the expanded notices cover the last rows until the pointer leaves. Fixes pingdotgg#19880
e7ed45a to
5004d02
Compare
|
Taking over this one. I’ll preserve the intended overlay behavior, rebuild the scroll reservation fix against current main, minimize the tests, and refresh the app evidence. |
|
Pushed 8379462, preserving the contributor commits. Two independent whole-PR reviews are clean; the real-app proof and focused 42-test suite pass. The body includes matched screenshots and recordings. The old linked-issue warning is a false positive: the maintainer decision explicitly accepts temporary coverage by expanded notices. The fix must preserve the collapsed reservation and avoid the later jump, rather than scroll on expansion. CI is still blocked by the unchanged WorkflowOpeningBlock browser highlighting test after one rerun (run): 8048/8049 tests pass. The first run also timed out in AttachmentFilePreview; that cleared on rerun. A fresh test agent ran both unchanged files together: 6/6 pass. The affected tests/components, highlighting and lazy preview code, configuration, and lockfile match main, and neither fixture mounts the changed composer. No timeout or unrelated source changes were added here. This remains unmerged and not CI-ready; the remaining human gate is resolving or rerunning that existing full-suite timing failure. |
This branch has not been deployed
2 participants
Expanding stacked composer notices increased timeline padding without immediately scrolling. The next work-row update then moved the thread by that extra padding, and collapse could move it back.
Measure notice growth above the collapsed peek strip and subtract it from the raw overlay height before rounding the timeline reservation. The existing resting-composer allowance and full-overlay clearance remain intact. Expanded notices intentionally cover the last rows, following the maintainer decision on #19880.
Continues @derektrimm's original #20086, preserving the contributor commits and merging main. Retained the useful measurement marker/minimum-height approach; simplified the rounded-height bridge into one subtract-then-round calculation.
Validation:
Before — main, expanded notices after the new work row:
After — head, same state:
Before — expand, new row, collapse:
Before recording
After — expand, new row, collapse:
After recording
Native macOS/Windows, Electron shell, iOS/Android, physical touch, and remote/relay/tunnel clients were not run. This is shared web/desktop layout code; native mobile has a separate composer. Two independent whole-PR reviews found no blockers. Current-head lint, build/typecheck, server tests and focused footer tests pass. Full Web CI remains blocked after one unchanged-head rerun: 8048/8049 tests pass; the unchanged WorkflowOpeningBlock short-script highlighting test exceeds its one-second wait. Both affected preview suites pass all six focused tests. This PR is not CI-ready; see the maintainer follow-up. Unmerged.
Implementation: Codex gpt-6.1-sol (high), tests: Codex gpt-6-astra (high), adversarial reviews: Claude claude-opus-5-5 and Codex gpt-6-sol, through T3 Code.